-
Notifications
You must be signed in to change notification settings - Fork 139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change port array in PROGRAMMER to be const char * #1699
Conversation
This changes the libavrdude API and ABI in an incompatible way, and therefore also requires a change to the library soname. All library users need to at least rebuild, possibly also change their source code. |
This is correct (though in a C++ world the |
I do not see any issues to change now since no real use of libavrdude as of now. |
The size of the PROGRAMMER struct and possibly/probably the memory layout of the other struct members has changed. So all binary code dynamically linked against libavrdude would access the wrong memory location, and it is better for the code to fail to load at the dynamic linking stage and not run at all than the code misinterpreting the data in memory at runtime. So the soname must change, both the BTW, changing the type and size of a private class member in C++ would also change the memory size and memory layout of the class, certainly breaking the ABI, and possibly breaking the API. The way to avoid that kind of thing is to make the type opaque, both the member layout and also the total size of the type, and only give interface users accessor functions. In C, you achieve that by using incomplete types and only giving API users pointers, never a complete type. |
|
@ndim Thanks for the tips! OK, if we adopt sth like semantic versoning for the library, we'd have to change the major version number. And good point about dynamic linking. We routinely change the structs as AVRDUDE gets better at modelling programmers and parts, so we are likely to see rapidly changing major version numbering (but that's the nature of trying to keep up with the world of AVR parts and Microchip's changes) |
For shared libraries on Linux, FreeBSD, etc. the cmake library VERSION 7.8.9 gives the real name, e.g. Also, if the libavrdude API and ABI are that unstable, perhaps at least the libavrdude.h file as the only documentation of the API should prominently mention that so that library users have to find it. As an aside, with the API and ABI that volatile, I will keep not shipping the libavrdude library in the Fedora avrdude package at this time. |
@ndim Sure, everyone compiling a program against
... and comment early on in
|
The value of |
Probably best then to increment soname and library version from now on every time the ABI/API changes. Let's see how often that actually happens. |
The port array in the PROGRAMMER structure is wasteful and can cause a buffer overflow by the way the command line
-P
string used to be just copied into the array. Here what happens if you have a 5000 character random-P
string:With this PR, the programmer structure is much smaller and no buffer overflow happens here: